Implement MoonLocation.of_site#50
Conversation
lpsinger
force-pushed
the
of_site
branch
2 times, most recently
from
b5c5bee to
f353bbe
Compare
|
This is ready for review. An important first question is if this is a useful definition of "of_site" or if height from the datum is essential. |
aelanman
left a comment
aelanman
left a comment
There was a problem hiding this comment.
Hi @lpsinger ; This looks like a very useful addition!
A couple notes:
- Does the site it's querying provide heights above datum for each selenodetic position? I assume not, since you left it an open question. I'm happy to skip that for now if it's unavailable.
- You should add at least one unit test to cover the new feature, and possibly update the github actions to cache the klm file per-OS. I can help with that if you need.
It does not.
I added a doctest. It is not cached. It was unclear how frequently this file is updated, so I didn't cache it. |
|
Thanks for adding the test. I just meant caching for the test suite (like how the kernel files are cached). This is done to reduce the download requests from 10 to 2 each time the testsuite workflow is triggered. |
Look up a surface feature in the USGS Gazetteer of Planetary Nomenclature (https://planetarynames.wr.usgs.gov/Page/MOON/target). - This data source provides the longitude and latitude, but not the height, of the approximate centers of craters and other features. - Locations are referenced to the IAU2000 selenoid. - This function downloads a KML file that is 11M in size the first time that you call it in a given Python session. Fixes aelanman#10.
|
#55 will fix the unit tests. |
aelanman
left a comment
aelanman
left a comment
There was a problem hiding this comment.
I'm happy to merge this now if you think it's ready. I don't want to drop support for older python versions just yet, since one of the main users of this package still supports older versions. Also, it looks like the namespace failure on path.glob is not just a result of python versioning. I'll see if I can fix the tests
2 participants
Look up a surface feature in the USGS Gazetteer of Planetary Nomenclature (https://planetarynames.wr.usgs.gov/Page/MOON/target).
Fixes #10.
Note: merge #49 first.